Skip to content

Conversation

@Kobzol
Copy link
Member

@Kobzol Kobzol commented Dec 3, 2024

ruff can both lint and format Python code (in fact, it should be a mostly drop-in replacement for black in terms of formatting), so it's not needed to use black anymore. This PR removes black and replaces it with ruff, to get rid of one Python dependency, and also to make Python formatting faster (although that's a small thing).

If we decide to merge this, we'll need to "reformat the world" - ruff is not perfectly compatible with black, and it also looks like black was actually ignoring some files before. I tried it locally (./x test tidy --extra-checks=py:fmt --bless) and it also reformatted some code in subtrees (e.g. clippy or rustc_codegen_gcc) - I'm not sure how to handle that.

@rustbot
Copy link
Collaborator

rustbot commented Dec 3, 2024

r? @onur-ozkan

rustbot has assigned @onur-ozkan.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

@rustbot rustbot added A-testsuite Area: The testsuite used to check the correctness of rustc S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) labels Dec 3, 2024
@jieyouxu jieyouxu added A-Python Area: Python, like bootstrap/CI scripts or linters/formatters, etc. A-testsuite Area: The testsuite used to check the correctness of rustc and removed A-testsuite Area: The testsuite used to check the correctness of rustc labels Dec 3, 2024
@jieyouxu
Copy link
Member

jieyouxu commented Dec 3, 2024

I think trimming the python deps (and also making py fmt slightly faster) is nice, will let Onur assess if the improvement is worth the churn of reformatting

Copy link
Contributor

@onur-ozkan onur-ozkan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we decide to merge this, we'll need to "reformat the world"

I liked the changes in this PR, how much change do you mean with "world"? Can you make a draft PR to show that?

@rustbot rustbot added the T-infra Relevant to the infrastructure team, which will review and decide on the PR/issue. label Dec 4, 2024
@rustbot
Copy link
Collaborator

rustbot commented Dec 4, 2024

Some changes occurred in src/tools/clippy

cc @rust-lang/clippy

The Miri subtree was changed

cc @rust-lang/miri

Some changes occurred in compiler/rustc_codegen_gcc

cc @antoyo, @GuillaumeGomez

@Kobzol
Copy link
Member Author

Kobzol commented Dec 4, 2024

I committed the change. It also modifies subtress though, e.g. rustc_codegen_gcc. We should probably add these to the ignore list in ruff.toml.

@onur-ozkan
Copy link
Contributor

We should probably add these to the ignore list in ruff.toml.

That or formatting them there first (would be pretty exhausting to file PR one by one).

--

I think the overall change is not too much and the changes are all in python files, so they likely won't cause so many conflicts with other ongoing tasks/PRs. We can land this easily once subtrees are handled.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Chances are if we ever edit this file in Miri it'll be formatted wrong... I know nothing about Python formatting.^^

But usually we exclude subprojects from formatting (e.g. via rustfmt).

@Kobzol Kobzol force-pushed the replace-black-with-ruff branch from e571e0c to 536516f Compare December 4, 2024 22:04
@Kobzol
Copy link
Member Author

Kobzol commented Dec 4, 2024

Ok, I ignored miri, clippy and rustc_codegen_gcc and ran formatting again.

@onur-ozkan
Copy link
Contributor

@bors r+

@bors
Copy link
Collaborator

bors commented Dec 5, 2024

📌 Commit 536516f has been approved by onur-ozkan

It is now in the queue for this repository.

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Dec 5, 2024
@bors bors merged commit 5a9c9ef into rust-lang:master Dec 6, 2024
6 checks passed
@rustbot rustbot added this to the 1.85.0 milestone Dec 6, 2024
@Kobzol Kobzol deleted the replace-black-with-ruff branch December 6, 2024 06:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-Python Area: Python, like bootstrap/CI scripts or linters/formatters, etc. A-testsuite Area: The testsuite used to check the correctness of rustc S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) T-infra Relevant to the infrastructure team, which will review and decide on the PR/issue.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants